Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

NativeBufferAllocator: release memory when buffer object is GCed #67

Merged
merged 60 commits into from
Apr 6, 2023
Merged

NativeBufferAllocator: release memory when buffer object is GCed #67

merged 60 commits into from
Apr 6, 2023

Conversation

Ali-RS
Copy link
Contributor

@Ali-RS Ali-RS commented Mar 27, 2023 •
edited
Loading

Fix #66

There are 2 TODOs remaining to be fixed.

@Ali-RS Ali-RS changed the title Fix memory leak NativeBufferAllocator: release memory when buffer object is GCed Mar 27, 2023
@pavly-gerges
Copy link
Member

Hello @Ali-RS, thanks for the PR.
If you would like to continue to fix this issue, please kindly follow the code design illustrated on issue #66, the code should be similar to the one on the issue (ofc with your own optimizations), and we need an example at jme3-alloc-examples to test the code.

@pavly-gerges pavly-gerges added core Concerning API core code incompatibility An incompatibility with the original target. incompatibility-patch Patch to fix the incompatibilities and removed incompatibility An incompatibility with the original target. labels Mar 27, 2023
@pavly-gerges pavly-gerges added this to the v1.0.0-gamma milestone Mar 27, 2023
@Ali-RS
Copy link
Contributor Author

Ali-RS commented Mar 27, 2023 •
edited
Loading

please kindly follow the code design illustrated on issue #66, the code should be similar to the one on the issue

Hi @Scrappers-glitch, this PR is designed based on the JME LWJGL 3 buffer allocator, I am not sure I can follow the approach illustrated on issue #66 as I do not understand it fully. Please feel free to close this PR if you prefer a different implementation. Thanks

@pavly-gerges
Copy link
Member

please kindly follow the code design illustrated on issue #66, the code should be similar to the one on the issue

Hi @Scrappers-glitch, this PR is designed based on the JME LWJGL 3 buffer allocator, I am not sure I can follow the approach illustrated on issue #66 as I do not understand it fully. Please feel free to close this PR if you prefer a different implementation. Thanks

No problem, I will try to continue on this PR and add the intended design.

@pavly-gerges pavly-gerges self-assigned this Mar 27, 2023
pavly-gerges and others added 16 commits March 27, 2023 18:48
@pavly-gerges
NativeBufferUtils: reverted code
48a31a1
@pavly-gerges
GarbageCollectibleBuffers: basic API
fcbdc14
@pavly-gerges
GarbageCollectibleBuffers: added jme interface
7ce8ea0
@pavly-gerges
NativeBufferUtils: added NativeBufferUtils#destroy(long)
03eb6df
@pavly-gerges
jmealloc-natives: added native implementation for `NativeBufferUtils#…
1a6201e 
…destroy(long)`
@pavly-gerges
jme-alloc-natives: fixed too few args error for destroy0(void*)
efe2d00
@pavly-gerges
com_jme3_alloc_util_NativeBufferUtils.c: fix `getMemoryAddress(JNIEnv…
3ce4d20 
…*, jobject*)` to return mem-address
@pavly-gerges
GarbageCollectibleBuffers: keep track of created buffer addresses to …
6e3c309 
…avoid duplicated actions
@pavly-gerges
API: refactored NativeBufferUtils#getMemoryAddress(Buffer) to use a…
d2fe2c2 
…bstract Buffer
@pavly-gerges
API: Code-Optimizations
8265ed9
@pavly-gerges
GarbageCollectibles-API: added loggers
4540458
@pavly-gerges
GarbageCollectibles: further optimizations by hiding the Collectibles…
0ecb303 
… MemoryScavenger
@pavly-gerges
GarbageCollectibles: made MemoryScavenger package-private
f4ee922
@pavly-gerges
GarbageCollectibles-API: refactored to OOP
1276740
@pavly-gerges
MemoryScavenger#LOGGER: removed static modifier
55b317e
@pavly-gerges
Merge branch 'fix-memory-leak' of https://github.com/Ali-RS/jme-alloc …
370983a 
…into fix-memory-leak
@pavly-gerges pavly-gerges linked an issue Mar 30, 2023 that may be closed by this pull request
[Deployment] Deploy a concomitant debug binary with each release #65
Closed
@pavly-gerges pavly-gerges temporarily deployed to maven-central March 30, 2023 15:30 — with GitHub Actions Inactive
@pavly-gerges pavly-gerges temporarily deployed to maven-central March 30, 2023 15:31 — with GitHub Actions Inactive
@pavly-gerges pavly-gerges temporarily deployed to maven-central March 31, 2023 09:25 — with GitHub Actions Inactive
@pavly-gerges pavly-gerges temporarily deployed to maven-central March 31, 2023 09:28 — with GitHub Actions Inactive
@pavly-gerges
Copy link
Member

@Ali-RS I would like to release the gamma version and the final stable version during this week, so please, could you help me by adding the examples you suggested on forums at the jme3-alloc-examples module, thank you.

@Ali-RS
Copy link
Contributor Author

Ali-RS commented Apr 2, 2023

I would like to release the gamma version and the final stable version during this week

I think we should not rush, there are still issues that need to be fixed.

See: https://hub.jmonkeyengine.org/t/jme-alloc-project/46356/180?u=ali_rs

@pavly-gerges
Copy link
Member

@Ali-RS Thanks for your contributions, this PR will be merged for now, if you want to inquire about the API status don't hesitate to ping me on the forums or here on a new issue, Have a great day!

@pavly-gerges pavly-gerges merged commit b098af9 into Electrostat-Lab:master Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pavly-gerges pavly-gerges pavly-gerges left review comments

Assignees

@Ali-RS Ali-RS

@pavly-gerges pavly-gerges

Labels
core Concerning API core code incompatibility-patch Patch to fix the incompatibilities
Projects
None yet
Milestone
v1.0.0-gamma
2 participants
@Ali-RS @pavly-gerges